Skip to content

feat: discovery-first tool resolution (make Mason optional)#21

Open
charliie-dev wants to merge 71 commits into
mainfrom
feat/discovery-first-tooling
Open

feat: discovery-first tool resolution (make Mason optional)#21
charliie-dev wants to merge 71 commits into
mainfrom
feat/discovery-first-tooling

Conversation

@charliie-dev

@charliie-dev charliie-dev commented Jun 30, 2026

Copy link
Copy Markdown
Owner

Feasibility POC — RFC ayamir/nvimdots#1293

Implements the "separate tool discovery from tool installation" idea from this comment: Mason becomes one installer backend, not a hard requirement. On FreeBSD/NixOS/etc. — where Mason's prebuilt binaries don't run — system-provided tools are used directly, with no manual symlinking and no startup notification spam.

No new toggle, no OS detection. Behaviour is driven purely by availability.

Resolution model (per tool)

0. unrecognized name ─────────────────► "fix your config" guidance (never installs)
1. binary on $PATH (system / Mason) ──► configure now, same tick
2. Mason ships a package ─────────────► install in background, configure on completion
3. self-validating local config ──────► configure now; its error() joins the warning
4. neither ───────────────────────────► aggregate into ONE "please install" warning

Applied to LSP, DAP, formatters, and linters through one shared loop (modules/utils/tools.lua). Everything resolvable without an install registers synchronously (lazy-load trigger replay still works); only installs defer.

Changes

Area File What
Helper modules/utils/tools.lua (new) Shared resolution loop: $PATH + Mason-bin-dir probe, install-then-configure tracking, bin→package reverse lookup, per-subsystem warning aggregator (one message, not N) with a configurable deadline (settings.tool_install_timeout)
LSP mason-lspconfig.lua, lsp.lua Cascade resolver driven by lsp_deps; external_lsp_deps merged into lsp_deps; function-cmd servers (e.g. jsonls) probed via the Mason package's declared bins; off-$PATH cmds rewritten to absolute paths before vim.lsp.enable() (covers Mason PATH = "skip")
Formatter/Linter conform.lua, nvim-lint.lua, mason.lua Deps resolved against each plugin's own runtime registry (the ground truth), Mason package reverse-looked-up by binary; mason.lua's ensure_installed loop removed (UI-only now)
DAP dap/init.lua, dap/clients/* Self-driven over dap_deps; mason-nvim-dap's private mappings guarded; client configs self-validate and resolve binaries lazily on the spawn path, so remote attach works without local binaries

settings.lua

  • lsp_deps: absorbs nil_ls, nixd, and shuck — one list, every entry is wanted. Package-less servers (nixd, shuck) are enabled from their $PATH binary; when absent they join the aggregated warning (the fix is provisioning via home-manager/mise, not Mason)
  • external_lsp_deps: removed
  • formatter_deps / linter_deps: entries are now the subsystem's tool names, matching lsp_deps semantics (cmakelangcmake_format, golangci-lintgolangcilint); the Mason package name is derived, not configured
  • tool_install_timeout (new): deadline before the aggregated warning is flushed despite unsettled installs

Verification

  • CI: stylua + selene + nix flake check, green.
  • Headless smoke: config loads and attaches lua_ls via the new resolver, no errors.
  • Copilot review loop run to convergence (all threads resolved, latest round: no new comments).

Known POC limitations

  • DAP $PATH detection leans on the Mason package's declared binaries (nvim-dap has no uniform command registry); adapters without a mason-nvim-dap mapping need a client config.
  • Not yet tested on an actual BSD/NixOS host — that's the next feasibility step.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements a discovery-first tool resolution model so that tools already available on $PATH (system-provided or Mason-provided) are used directly, while Mason becomes an optional installer backend rather than a hard runtime gate. This supports environments like NixOS/FreeBSD where Mason-installed binaries may be unavailable or undesirable.

Changes:

  • Add shared helpers for $PATH probing, Mason spec.bin extraction, and aggregated “missing tools” warnings.
  • Rework LSP and DAP setup to be driven by desired dependency lists with discovery-first resolution and deferred installs.
  • Update Mason bootstrap for formatters/linters to only install when the tool isn’t already on $PATH, aggregating missing-tool warnings.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
lua/modules/utils/tools.lua New helper module for executable discovery + aggregated missing-tool warnings.
lua/modules/configs/tool/dap/init.lua Switch DAP setup to discovery-first resolution instead of mason-nvim-dap’s installed-only handler gating.
lua/modules/configs/completion/servers/shuck.lua Update server comment to reflect the new discovery-first resolution model.
lua/modules/configs/completion/mason.lua Only install formatter/linter packages when not already available on $PATH; aggregate missing warnings.
lua/modules/configs/completion/mason-lspconfig.lua Drive LSP setup via discovery-first resolution + deferred installs, using mason-lspconfig mappings.
lua/modules/configs/completion/lsp.lua Remove external_lsp_deps wiring; rely on centralized discovery-first LSP setup.
lua/core/settings.lua Collapse external/system LSP list into unified lsp_deps list with discovery-first semantics.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lua/modules/configs/completion/mason-lspconfig.lua Outdated
Comment thread lua/core/settings.lua Outdated
Comment thread lua/modules/configs/tool/dap/init.lua Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread lua/modules/configs/completion/mason-lspconfig.lua Outdated
Comment thread lua/modules/configs/tool/dap/init.lua Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comment thread lua/modules/configs/tool/dap/init.lua Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread lua/core/settings.lua Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Comment thread lua/modules/utils/tools.lua Outdated
Comment thread lua/modules/configs/tool/dap/init.lua Outdated
Comment thread lua/modules/configs/tool/dap/init.lua Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Comment thread lua/modules/configs/completion/mason-lspconfig.lua Outdated
Comment thread lua/modules/configs/completion/mason.lua Outdated
Comment thread lua/core/settings.lua Outdated
Comment thread lua/modules/utils/tools.lua Outdated
Comment thread lua/modules/utils/tools.lua Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread lua/modules/utils/tools.lua Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread lua/modules/utils/tools.lua Outdated
…bugpy

the config-time availability check ran the full debugpy cascade, including
the blocking vim.system():wait() import probe, on the shared :Dap tick
where the resolver validates every dap_deps client — debugging go paid
200-500ms for python's probe whenever the debugpy-adapter shim was absent.

the first shimless check now raises without spawning when the raise can
actually provision: both mason.nvim and mason-nvim-dap.nvim are
lazy-managed with their code on disk, the mapper still derives a package
for python, and mason-registry resolves that package — every link
pcall-guarded and degrading to the probe path on any drift. the raise is
one-shot per process (module-level latch): if provisioning does not
produce the shim, every later validates run falls back to the bounded
import probe, so installed-but-shimless and mason-less pip setups keep
their recovery and a capable system python still passes without a false
missing-warning. launch-path semantics (ttl/budget negative cache) are
untouched.
vim.uv.cwd() returns nil when the process working directory has been
deleted; the "Debug (using venv)" pythonPath concatenated it unchecked
and aborted the launch with a concat error. a dead cwd now degrades to
the interpreter-name fallback.
package_of called mason_lspconfig.get_mappings() bare while every sibling
registry touch in the file is pcall'd; a drifted or missing get_mappings
threw into the resolver's outer pcall, error-marking every off-$PATH
server with a raw lua error string. the read is now pcall'd and typed
down to mappings.lspconfig_to_package being a table, degrading to the
empty map — $PATH-only classification with the normal missing/unknown
report, re-fetch-while-empty semantics preserved.
attach_registry_events wrapped both registry:on subscriptions in one
pcall deriving one flag: a drift throw from the second subscription
after the first registered left the flag false, and the next attach
registered the install handler a second time — duplicate retry_pending
walks on every later install. each subscription now has its own pcall
and its own flag: a succeeded half never re-registers, a failed half
retries on the next attach, and total failure keeps the status quo.
callback bodies unchanged.
the remote-attach branch validated port strictly but passed config.host
through untouched, so a non-string or empty host surfaced as an opaque
connection failure instead of the clear config-time error the port
already gets. host now mirrors the port contract for shape only (hosts
are free-form, so no format guessing): nil keeps the 127.0.0.1 default,
anything else must be a non-empty string.
mason_root evaluated its sources once: a pre-mason-load first call with
$MASON set cached the env value permanently, so the documented
higher-priority mason.settings.install_root_dir was never adopted once
mason loaded — call timing inverted the priority. worse, a loaded-but-
not-set-up mason.settings (mason-registry requires it with the DEFAULTS;
only setup() applies a custom root) could outrank $MASON.

the settings source is now read live each call, gated on
mason.has_setup: presence decides the branch, existence decides the
return (a configured-but-uncreated root yields nil, never a fallback to
a wrong root). $MASON stays cached as mason_env_root; the data-dir
guess and the user-override memo are unchanged.
… call

the mason_path_added flag short-circuited before the membership check,
so a $PATH overwritten mid-session was never healed and a root that
switched once mason.setup applied a different one never got its bin
appended. the flag is gone: the exact-entry membership check each call
is the idempotence (one plain find per resolve run).

append-only on purpose: identical strings in an externally mutable list
admit no robust ownership proof, so nothing is ever removed — and per
mason PATH mode nothing needs to be (prepend re-orders, append is tail
ordering by choice). mason PATH="skip" remains a documented deviation:
the append keeps availability classification and bare-name spawns in
agreement; honoring skip coherently needs absolute-command
configuration in every consumer, tracked as follow-up.
the "never load mason-registry on a provisioned setup" thunk existed as
byte-identical copies (code and comment) in the dap resolver spec and in
resolve_runtime_tools. tools.default_registry is now the one copy both
specs reference, so a change to the load guard cannot drift between
consumers. spec.registry semantics unchanged: still a value-or-thunk,
still only invoked in phase 2.
run() re-inlined the thunk resolution that session.resolve_registry
already owns, and the inline copy lacked the final table check: a
drifted thunk returning a non-table crashed phase 2 outright
("attempt to index local registry (a boolean value)"), aborting the
whole resolve pass. run() now resolves through the session helper, so
a non-table result degrades to the nil-registry path — a clean
aggregated missing report instead of a crash.
resolve_deps hand-rolled the valid/invalid dep classification with
table.maxn although tools.split_dep_names is the one definition of what
counts as a valid dep entry and nvim-lint already delegates to it. the
container-shape policy stays here (a non-table lsp_deps is dropped
whole, exactly as before); invalid entries now land at the immediate
batch's tail instead of interleaved — unobservable, the collector sorts
its report and classification is per-name.
the "table cmd -> cmd[1] as the probeable binary" extraction existed
five times across server_info and the cache invalidator with varying
outer guards. binary_of_cmd is now the single rule; the self_resolving
branch keeps {} distinct from a function cmd (nil binary without the
flag), byte-equivalent to the old structure.
…aths

the mason install-event handler and :ToolsRetry both probed a pending
name's declared bare binaries with the same pcall(binaries_of) +
find_executable core. pending_bins_available is now that one core; the
is_unsettled ownership guards stay per call site because the two paths
interleave different extra evidence around the probe, and the evidence
order is unchanged.
the type(x) == "string" and x or nil sanitization of untrusted
probe/config fields appeared four times (the validates record and the
three runtime-tools probe-result fields). str_or_nil is now the single
sanitizer; the collector's non-empty-string variant is a different rule
and stays inline.
the single-return wrapper (round-3 g8) protected vararg-position callers
from split_dep_names' second return, but every live call site truncates
fine without it. reader audit at removal time:
- repo (rg 'normalize_names' lua/): the wrapper, its M alias, two
  internal single-assignment calls (find_executable, exepath_or_error),
  one external call site (mason-lspconfig lsp_deps set, ipairs position,
  now parenthesized), one comment reference (updated).
- fleet harnesses (rg over /tmp/*smoke*/run.lua): zero hits.
- user overlay: lua/user/ exists (gitignored surface) and is currently
  empty — zero files, zero possible readers.
callers use split_dep_names directly; the one vararg-position site keeps
its truncation via parentheses.
the export has no external runtime caller today; it stays public as the
tested contract of the has_setup-gated root-priority semantics, pinned
directly by the g4-latch and collector harnesses. the docstring now says
so, making the surface self-justifying in source.
the handler rebuilt the 2-element client_modules table on every call
only to compare its first entry against load_client_config's winner
string. the loader now records user_won at cache-build time, where the
module list already exists, and returns the boolean instead — winner
had no other consumer.
resolve_remaining's docstring claimed the parity sweep calls the export;
the sweep calls the local upvalue — the export is the manual escape
hatch and the harness hook, and now says exactly that. the
eager_ft_override_modules export gains its test-hook label (read by the
ft_override_consistency harness scenario; no runtime reader), so both
surfaces are self-justifying in source.
Reattach the attach_registry_events and package_for_binary docstrings to
their actual definitions, add the missing is_unsettled field to the
ToolCollector class annotation, and update the phase-2 comments for LSP's
defer_phase2 adoption. Compress the PATH-mode narrative behind a mason 2.x
semantics anchor, note the async-emit immunity on the update handler, and
state that default_registry is only resolved once leftovers exist. The
mason_root test-hook label now explains WHY it stays public instead of
naming harness scenarios.
Rewrite the user_lsp_configs replay comments around the actual ordering
guarantee (discovery runs after the user module), fix the stale
server_info/invalidate reference, and swap the per-machine verification
claim for the documented :h vim.lsp.enable() semantics. Mark SWEEP_DELAY_MS
as kept in sync with nvim-lint's parity sweep. shuck's header now records
mise as a deliberate choice: Mason ships a shuck package these days, but
the $PATH copy wins under discovery-first; the zsh-dialect caveat gains a
version anchor so it can expire.
The markdownlint-cli2 override rationale dated from the bun node-shim era;
record that mise ships real node now (kept pending re-evaluation) and NOTE
the known limitation that the pattern drops col-less findings (e.g. MD012).
Correct the shuck comment: actionlint DOES lint run: blocks via its
shellcheck pass-through — shuck complements it. Consolidate the
golangcilint blocking-load fact onto refresh_linter and point the other
two sites at it; the rawset comment no longer cites the removed
factory-wrapper machinery.
The prettier --write-on-temp-copy rationale dated from the bun node-shim
era; record that mise ships real node now (kept pending re-evaluation).
Drop the two comments restating the format_on_save code and the null-ls
migration attribution.
Name the user-override materialization as the second mason-nvim-dap load
site, correct the upstream default_setup ipairs claim, and point the
validate-order notes in every client back at the canonical contract in
init.lua. delve: 38697 is the nvim-dap wiki's example port, not a delve
default. lldb: the lldb-dap rename landed in LLVM 18, not 15. python:
cover all raise sites and drop two boilerplate configuration comments.
palette_overwrite is a name -> palette map, not a list: fix the @type.
Soften the nil_ls/nixd/shuck notes into provisioning-preference statements
instead of Mason-registry claims that rot as the registry changes, drop
the duplicated timeout literal and scheduled-notify rationale, and note
the debug keymaps as a dap lazy-load trigger.
The dead block referenced telescope, which left this config in 2026-03.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Comment thread lua/modules/configs/completion/nvim-lint.lua

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Comment thread lua/core/settings.lua
palette_overwrite is a PARTIAL override merged via tbl_extend(force), but
the palette class declares every field required, so the corrected @type
made lua_ls flag the empty default with missing-fields. Keep the class
annotation (it drives the advertised key completion) and disable that one
diagnostic on the assignment; verified with lua-language-server --check
that the suppression line must sit directly above the assignment.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Comment thread lua/modules/configs/tool/dap/clients/python.lua
The probe runs synchronously on the :Dap* setup tick; a hung interpreter
(dead network FS, broken shim) would block the editor indefinitely. wait(5000)
SIGKILLs the probe on timeout with exit code 124 (:h SystemObj:wait()), which
the existing non-zero check already treats as a failed probe, so resolution
falls through to the remaining candidates.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants